Skip to content

Conversation

@InJunKangW
Copy link
Collaborator

📌 PR 제목

마이 페이지 기능 추가

✨ 변경 사항

  1. 내가 작성한 다이어리 조회
  2. 내가 좋아요 누른 다이어리 조회
  3. 내 팔로워 조회
  4. 내 팔로잉 조회
  5. 내 결제 정보 조회

🔍 변경 이유

✅ 체크리스트

  • 코드가 정상적으로 동작하는지 확인
  • 관련 테스트 코드 작성 및 통과 여부 확인
  • 문서화(README 등) 필요 여부 확인 및 반영
  • 리뷰어가 알아야 할 사항 추가 설명

📸 스크린샷 (선택)

📌 참고 사항

  • 결제의 경우 기능을 뺀다고 이야기를 했었는데 어느 정도 간략히 작성을 해놨어서 일단은 추가한 상태로 두겠습니다.

@github-actions
Copy link

github-actions bot commented Apr 5, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • 페이지네이션 로직을 PageableUtil 클래스로 분리하여 재사용성을 높였습니다.
  • 구독 관련 기능을 추가하여 사용자 경험을 향상시켰습니다.
  • MyPageService를 통해 마이페이지 관련 기능을 중앙화하여 관리하기 쉽게 만들었습니다.

주요 이슈:

  1. DiaryService의 책임 분리

    • 제안: DiaryService의 일부 책임을 새로운 LikeService로 분리하여 단일 책임 원칙을 준수하세요.
    @Service
    public class LikeService {
        private final DiaryRepository diaryRepository;
    
        public Long incrementLikeCount(Long diaryId) {
            Diary diary = diaryRepository.findById(diaryId)
                .orElseThrow(NotFoundDiaryException::new);
            return diary.incrementLikeCount();
        }
    
        // 다른 좋아요 관련 메서드들...
    }
  2. MyPageService의 중복 코드

    • 제안: getMyFollowers와 getMyFollowings 메서드의 중복 코드를 제거하세요.
    private PageResponse<UserThumbnailResponseDto> getFollowSlice(Long userId, Long cursorId, Function<Long, Long, PageRequest, Slice<UserThumbnailResponseDto>> sliceFunction) {
        Slice<UserThumbnailResponseDto> slice = sliceFunction.apply(userId, cursorId, PageRequest.of(0, defaultPageSize));
        Long nextCursor = !slice.isEmpty() ? slice.getContent().getLast().userId() : null;
        return PageResponse.of(slice, nextCursor);
    }
    
    public PageResponse<UserThumbnailResponseDto> getMyFollowers(Long userId, Long cursorId) {
        return getFollowSlice(userId, cursorId, followQuerydsl::getFollowerSliceByUserId);
    }
    
    public PageResponse<UserThumbnailResponseDto> getMyFollowings(Long userId, Long cursorId) {
        return getFollowSlice(userId, cursorId, followQuerydsl::getFollowingSliceByUserId);
    }
  3. CustomDiaryRepositoryImpl의 복잡성

    • 제안: 쿼리 조건 생성 로직을 별도의 메서드로 분리하여 가독성을 높이세요.
    private BooleanExpression createLikeDiaryCondition(Long userId, List<VisibilityType> visibilities, Long cursorId) {
        return like.userId.eq(userId)
            .and(diary.visibility.in(visibilities))
            .and(like.likeId.lt(cursorId));
    }
    
    @Override
    public Slice<Diary> getLikeDiarySliceByUserId(Long userId, List<VisibilityType> visibilities, Long cursorId, Pageable pageable) {
        List<Diary> content = queryFactory
            .selectFrom(diary)
            .innerJoin(like)
            .on(like.diaryId.eq(diary.diaryId))
            .where(createLikeDiaryCondition(userId, visibilities, cursorId))
            .orderBy(like.createdAt.desc())
            .limit(pageable.getPageSize() + 1)
            .fetch();
    
        return checkAndCreateSlice(content, pageable);
    }

관련 파일에 대한 영향 분석:

  • DiaryService의 변경은 LikeService와 CommentService에 영향을 줄 수 있습니다. 이들 서비스 간의 의존성을 주의깊게 관리해야 합니다.
  • MyPageService의 변경은 MyPageController에 직접적인 영향을 미칩니다. 컨트롤러의 메서드 시그니처나 반환 타입이 변경될 수 있습니다.
  • CustomDiaryRepositoryImpl의 변경은 DiaryService와 MyPageService에 영향을 줄 수 있습니다. 쿼리 결과의 일관성을 유지해야 합니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었습니다. 특히 페이지네이션과 구독 기능의 추가는 좋은 개선입니다. 하지만 일부 서비스의 책임이 과도하게 커진 것 같습니다. 서비스 계층의 책임을 더 세분화하고, 중복 코드를 제거하면 유지보수성이 더욱 향상될 것입니다.

@github-actions
Copy link

github-actions bot commented Apr 7, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • 코드 전반적으로 구조화가 잘 되어 있으며, 특히 MyPageService의 구현이 체계적입니다.
  • CustomOAuth2User를 통한 인증 처리가 일관성 있게 적용되었습니다.
  • 페이징 처리를 위한 PageableUtil 클래스의 도입은 코드 재사용성을 높였습니다.

주요 이슈:

  1. DiaryService의 책임 과중

    • 제안: DiaryService의 일부 기능을 새로운 서비스 클래스로 분리하여 단일 책임 원칙을 준수하세요.
    public class DiaryQueryService {
        public PageResponse<DiaryResponseDto> getMyDiariesByCursor(Long userId, VisibilityType visibilityType, Long cursorId, int size) {
            // 현재 DiaryService의 getMyDiariesByCursor 메소드 내용
        }
    }
  2. FollowQuerydsl의 중복 코드

    • 제안: getFollowerSliceByUserId와 getFollowingSliceByUserId 메소드의 중복 로직을 추출하여 재사용성을 높이세요.
    private Slice<UserThumbnailResponseDto> getFollowSlice(boolean isFollower, Long userId, Long cursorId, Pageable pageable) {
        // 공통 로직 구현
    }
  3. MyPageServiceTest의 테스트 케이스 개선

    • 제안: 경계 조건과 예외 상황에 대한 테스트를 추가하여 테스트 커버리지를 높이세요.
    @Test
    void getMySubscription_whenSubscriptionExpired_returnsInactive() {
        // 만료된 구독에 대한 테스트 케이스 구현
    }

관련 파일에 대한 영향 분석:

  • DiaryService의 변경은 CommentService, LikeService 등 다른 서비스 클래스에 영향을 줄 수 있습니다. 이들 클래스에서 DiaryService를 사용하는 부분을 주의깊게 검토해야 합니다.
  • CustomDiaryRepository와 CustomDiaryRepositoryImpl의 변경은 DiaryRepository 인터페이스에 영향을 줄 수 있으므로, 이를 구현하는 다른 클래스들도 확인이 필요합니다.
  • MyPageService의 변경은 MyPageController에 직접적인 영향을 미치므로, 컨트롤러의 엔드포인트와 반환 타입이 일치하는지 확인해야 합니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었습니다. 특히 인증 처리와 페이징 로직의 개선이 눈에 띕니다. 다만, 일부 서비스 클래스의 책임이 과도하게 커진 점과 테스트 케이스의 보완이 필요해 보입니다. 이러한 부분들을 개선하면 코드의 유지보수성과 확장성이 더욱 높아질 것입니다.

Copy link
Collaborator

@TTaiJin TTaiJin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!
인준님이 구현하신 MyPageService에서 제가 구현한 DiaryService 메서드를 사용하는게 있는데 제가 리팩토링하면서 좀 변경해가지고 그 부분만 병합하고 수정하면 될 거 같습니다!

Copy link
Collaborator

@dnzp75 dnzp75 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 작성해주신것 같습니다 고생하셨습니다!

Pageable pageable
);

Slice<Diary> getLikeDiarySliceByUserId(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 일관성을 위해 서비스 층에서는 get~~ Repository층에서는 find~~ 메서드명을 이런식으로 맞추자라는 의견이 있었던 것 같은데, 혹시 맞을까요?

맞다면 메서드명 통일 시키는 것은 어떻게 생각하시나요?

@Transactional(readOnly = true)
public PageResponse<DiaryResponseDto> getMyDiariesByCursor(Long userId, VisibilityType visibilityType,
Long cursorId, int size) {
List<VisibilityType> visibilities =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용자 흐름에서 VisibilityType이 null로 들어오는 경우가 있을까요?

3개의 타입에 대해서 전부 일일이 찾는 비용이 성능을 많이 잡아먹지 않을까?라는 생각과, 아예 Type을 지정해서 받는게 특정 Type만 찾는다는 점에서 성능에 좀 더 유리하지 않나? 라는 생각이 들었던 것 같습니다

public Slice<UserThumbnailResponseDto> getFollowerSliceByUserId(Long userId, Long cursorId, Pageable pageable) {
boolean isFollowerQuery = true;
List<UserThumbnailResponseDto> content = getContent(isFollowerQuery, userId, cursorId);
return PageableUtil.checkAndCreateSlice(content, pageable);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 함수안에 있는 로직을 저는 로직단에서 일일이 작성을 했었는데 Util클래스로 빼는게 가독성, 재사용성면에서 훨씬 좋은 것 같네요. 배워갑니다

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 8, 2025

@dnzp75 dnzp75 merged commit 6ec10e0 into develop Apr 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants